Runtime APIs for node-side code #1401

rphmeier · 2020-07-13T23:57:56Z

No description provided.

rphmeier · 2020-07-14T01:15:22Z

As an integration plan, I'd suggest adding these runtime APIs alongside the existing ones, postfixing all the old ones with _Old as a first PR, and then having follow-up code that removes dependence on these deprecated APIs. Then have follow-on PRs for different subsystems to migrate them to the new set of APIs.

coriolinus · 2020-07-15T07:39:28Z

roadmap/implementers-guide/src/runtime-api/README.md

+Yields the validator-set at the state of a given block. This validator set is always the one responsible for backing parachains in the child of the provided block.
+
+```rust
+fn validators() -> Vec<ValidatorId>;


Suggested change

fn validators() -> Vec<ValidatorId>;

fn validators(parent_hash) -> Option<Vec<ValidatorId>>;

Without the parent hash, we're not providing a context from which to get a validator set; the runtime can't know whether we're requesting the current set, the set for the next block, or a historical set.

The Option is meant to handle the case that a validator set is unavailable: an invalid or unknown parent_hash, for example.

hm, by that logic we should make all of these Optional. I would rather bake in an assumption that the parent hash is valid as that's closer to what the underlying code provides. Invalid parent-hashes just return runtime API errors.

coriolinus · 2020-07-15T07:42:27Z

roadmap/implementers-guide/src/runtime-api/README.md

+impl GroupRotationInfo {
+	/// Returns the index of the group needed to validate the core at the given index,
+	/// assuming the given amount of cores/groups.
+	fn group_for_core(core_index: usize, cores: usize) -> usize;


Suggested change

fn group_for_core(core_index: usize, cores: usize) -> usize;

fn group_for_core(core_index: ValidatorIndex, core: usize) -> usize;

If we have a ValidatorIndex newtype, we may as well use it.

But these aren't validator indices, they're core indices. We do have a CoreIndex type, although I would like to keep it private to the scheduler module. It drifted in #1312 ; after #1411 we can move them back a bit.

coriolinus · 2020-07-15T07:47:03Z

roadmap/implementers-guide/src/runtime-api/README.md

+	/// If this core is freed by being timed-out, this is the assignment that is next up on this
+	/// core. None if there is nothing queued for this core or there is no possibility of timing
+	/// out.
+	next_up_on_time_out: Option<ScheduledCore>,


Having two next_up_on fields feels like it introduces the potential for confusion. When would we want next_up_on_time_out to differ from next_up_on_available? If they do differ, is it possible for a core both to timeout and to become available simultaneously? If so, what's the actual next scheduled core?

"the runtime has all the answers".

We don't want them to differ, and yes, it is confusing, but this is purely exposing information about the system described in the scheduler module. It would be nice to have a clear "next up" in all cases, but deeper reading on the scheduler module will reveal why that is not possible.

is it possible for a core both to timeout and to become available simultaneously

No, they are mutually exclusive. These are the two different paths an Occupied core can take to the Free state. However, depending on which path is taken (which is unpredictable as it's based on the view/honesty of the block producer), the scheduling metadata can be different, leading to a different assignment onto the now-Free core.

In general we should always optimize for the next_up_on_available case in Node-side code. The reason being that timeouts are only possible at a small subset of blocks, as they are only triggered within the short span of time directly following a group rotation. And even when timeouts can be triggered, unless validators are offline, they will not be reached before availability. And if validators are offline, it's fine to degrade throughput of paras somewhat.

However this runtime API is in the interest of making all information available to the node so more advanced strategies can be taken as research evolves.

roadmap/implementers-guide/src/runtime-api/README.md

coriolinus · 2020-07-15T07:54:22Z

roadmap/implementers-guide/src/types/candidate.md

@@ -7,6 +7,14 @@ In a way, this entire guide is about these candidates: how they are scheduled, c

 This section will describe the base candidate type, its components, and variants that contain extra data.

+## Para Id
+
+A unique 32-bit identifier referring to a specific para (chain or thread). The relay-chain runtime guarantees that `ParaId`s are unique for the duration of any session, but recycling and reuse over a longer period of time is permitted.


It doesn't matter for the purpose of this PR, but I'm curious: are the wrapped u32s hashes of some kind, or handed out sequentially, or what?

We haven't described the registrar in the guide yet, but I think it gives them out sequentially. Parachain IDs start at 0 and parathread IDs start with some of the higher bits set, although I don't remember exactly what.

Reuse is fine as long as it's at least a few sessions apart, although it would be best not to reuse until having cycled completely. We could alternatively use a generation/index system like this for handing out IDs to avoid reuse over a long period of time: http://bitsquid.blogspot.com/2014/08/building-data-oriented-entity-system.html

The uniqueness property is what's important here and the details of the registrar are free to change

Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com>

ordian

Looks good!

I have a question about error handling though. None of the proposed runtime APIs return a Result. Question: at which layer a storage access error is handled and how or is it always infallible?

ordian · 2020-07-16T14:19:28Z

roadmap/implementers-guide/src/runtime-api/README.md

+/// Returns the validator groups and rotation info localized based on the block whose state
+/// this is invoked on. Note that `now` in the `GroupRotationInfo` should be the successor of
+/// the number of the block.
+fn validator_groups(at: Block) -> (Vec<Vec<ValidatorIndex>>, GroupRotationInfo);


would be nice to have a type alias for a validator group as well

type ValidatorGroup = Vec<ValidatorIndex>;

rphmeier · 2020-07-16T15:24:49Z

roadmap/implementers-guide/src/runtime-api/README.md

+/// Fetch the value of the runtime API at the block.
+///
+/// Definitionally, the `at` parameter cannot be any block that is not in the chain.
+/// Thus the return value is unconditional. However, for in-practice implementations


@ordian Does this address your Result question?

partially, I should have been more explicit in my question, this was more substrate related question than to this PR, namely how low-level db errors are handled. And from what I see, https://github.com/paritytech/substrate/pull/3997/files, either the errors will be swallowed or a panic will occur. Anyway, this is handled on a different level.

rphmeier · 2020-07-17T00:34:38Z

Will merge this as it is directionally correct and address follow-ons in #1411

rphmeier added 3 commits July 10, 2020 19:00

create a README on Runtime APIs

b06d5d9

add ParaId type

5e3dc85

write up runtime APIs

8651e37

rphmeier added A3-in_progress Pull request is in progress. No review needed at this stage. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Jul 13, 2020

github-actions bot added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Jul 13, 2020

rphmeier added 3 commits July 13, 2020 20:27

more preamble

539b7ba

rename

ac39016

rejig runtime APIs

8eebbe8

rphmeier added A3-in_progress Pull request is in progress. No review needed at this stage. A0-please_review Pull request needs code review. and removed A0-please_review Pull request needs code review. A3-in_progress Pull request is in progress. No review needed at this stage. labels Jul 14, 2020

rphmeier marked this pull request as draft July 14, 2020 15:39

add occupied_since to BlockNumber

8c8193d

rphmeier marked this pull request as ready for review July 14, 2020 22:57

rphmeier added 3 commits July 14, 2020 19:43

improve group_for_core

2456ae6

improve docs on availability cores runtime API

00f7446

guide: freed -> free

fde221b

rphmeier mentioned this pull request Jul 15, 2020

Implement Runtime APIs #1411

Merged

coriolinus reviewed Jul 15, 2020

View reviewed changes

rphmeier and others added 2 commits July 15, 2020 13:47

Update roadmap/implementers-guide/src/runtime-api/README.md

69c0d55

Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com>

add explicit block parameter to runtime API fns

eb9971e

ordian reviewed Jul 16, 2020

View reviewed changes

rphmeier commented Jul 16, 2020

View reviewed changes

montekki mentioned this pull request Jul 16, 2020

Availability store subsystem guide #1424

Merged

rphmeier merged commit d656215 into master Jul 17, 2020

rphmeier deleted the rh-guide-runtime-apis branch July 17, 2020 00:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Runtime APIs for node-side code #1401

Runtime APIs for node-side code #1401

rphmeier commented Jul 13, 2020

rphmeier commented Jul 14, 2020

coriolinus Jul 15, 2020

rphmeier Jul 16, 2020

coriolinus Jul 15, 2020

rphmeier Jul 15, 2020

coriolinus Jul 15, 2020

rphmeier Jul 15, 2020

rphmeier Jul 15, 2020

rphmeier Jul 15, 2020

coriolinus Jul 15, 2020

rphmeier Jul 15, 2020

ordian left a comment

ordian Jul 16, 2020

rphmeier Jul 16, 2020

ordian Jul 16, 2020

rphmeier commented Jul 17, 2020

	fn validators() -> Vec<ValidatorId>;
	fn validators(parent_hash) -> Option<Vec<ValidatorId>>;

	fn group_for_core(core_index: usize, cores: usize) -> usize;
	fn group_for_core(core_index: ValidatorIndex, core: usize) -> usize;

Runtime APIs for node-side code #1401

Runtime APIs for node-side code #1401

Conversation

rphmeier commented Jul 13, 2020

rphmeier commented Jul 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ordian left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rphmeier commented Jul 17, 2020